-
Notifications
You must be signed in to change notification settings - Fork 119
feat: use internal API for more organized implementations #412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Here's what I've tested locally ( I put a helper script into our ✅ Available Providers (25): 🔑 Missing API Keys (7): |
| @staticmethod | ||
| def _convert_completion_response(response: "Message") -> ChatCompletion: | ||
| """Convert Anthropic Message to OpenAI ChatCompletion format.""" | ||
| return _convert_response(response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about moving all the logic from the utils.py file into this function, but I ended up keeping it like this because it seemed smoother and kept the anthropic file from getting too busy 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will not complain about smaller files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that there are some provider-specific quirks, but it would be super great if we could make those happen in an inherited call with stuff before and after a super().acompletion(*args, **kwargs)
|
In a sense, I get that this is a bit of a YOLO: I updated all the providers and ran the integration tests, but its definitely still possible that I messed something up. However, this at least migrates us to a working setup, and if the integration tests are all passing I feel like that provides at least a moderate level of confidence that everything's ok. |
| @staticmethod | ||
| def _convert_completion_response(response: "Message") -> ChatCompletion: | ||
| """Convert Anthropic Message to OpenAI ChatCompletion format.""" | ||
| return _convert_response(response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will not complain about smaller files
| yield _create_openai_chunk_from_anthropic_chunk(event, kwargs.get("model", "unknown")) | ||
| yield self._convert_completion_chunk_response(event, model_id=kwargs.get("model", "unknown")) | ||
|
|
||
| async def acompletion( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we're standardizing API by adding more required methods, should we abstract acompletion et al. to provided methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea. This PR is already a big boy, so I'll split this off into a separate PR, so that this change doesn't keep exploding into a bigger change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @staticmethod | ||
| def _convert_completion_response(response: "Message") -> ChatCompletion: | ||
| """Convert Anthropic Message to OpenAI ChatCompletion format.""" | ||
| return _convert_response(response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that there are some provider-specific quirks, but it would be super great if we could make those happen in an inherited call with stuff before and after a super().acompletion(*args, **kwargs)
Description
Better structuring for providers where things are organized into shared function definitions
PR Type
💅 Refactor ## Relevant issuesChecklist